executor: add ordered window builder by hawkingrei · Pull Request #68480 · pingcap/tidb

hawkingrei · 2026-05-19T04:00:37Z

What problem does this PR solve?

Issue Number: ref #67989

Problem Summary:

This PR keeps only the executor-side building block needed by follow-up ordered-window planning work. It intentionally does not include planner enumeration, index-join fusion, or partition TopN pruning.

What changed and how does it work?

Add OrderedWindowExec as a thin wrapper over PipelinedWindowExec.
Add PipelinedWindowExec.OpenSelf() so builder paths that already opened children can initialize only window-local state.
Add BuildOrdered(...), which forces the pipelined window executor and leaves the ordered-input guarantee to the caller.
Add direct executor coverage for building and running the ordered-window wrapper while tidb_enable_pipelined_window_function is disabled.

Check List

Tests

Unit test
Integration test
Manual test (add detailed scripts or steps below)
No need to test
- I checked and no code files have been changed.

Commands:

make bazel_prepare
git diff --check

Additional validation:

The focused executor test passed before the naming-only follow-up.

Local limitation:

The renamed focused test, the full package test, and the Bazel target were attempted but blocked by local disk exhaustion (No space left on device) while writing temporary build outputs.

Side effects

Performance regression: Consumes more CPU
Performance regression: Consumes more Memory
Breaking backward compatibility

Documentation

Release note

Please refer to Release Notes Language Style Guide to write a quality release note.

None

Summary by CodeRabbit

New Features
- Optimized window function execution with improved handling for scenarios where input data is already properly partitioned and sorted according to window requirements.
Tests
- Comprehensive test coverage added for window execution functionality to validate correctness across various data scenarios.

coderabbitai · 2026-05-19T04:00:59Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d7b45789-dbb1-4737-9dfb-226e61fb0b9a

📥 Commits

Reviewing files that changed from the base of the PR and between a62a6d1 and 4f7242d.

📒 Files selected for processing (4)

pkg/executor/windows/BUILD.bazel
pkg/executor/windows/builder.go
pkg/executor/windows/pipelined_window.go
pkg/executor/windows/window_executor_test.go

📝 Walkthrough

Walkthrough

This PR introduces OrderedWindowExec, a new executor for window functions when child data already satisfies partition and order requirements. It refactors PipelinedWindowExec initialization by extracting setup logic into OpenSelf(), adds the BuildOrdered() builder function, and includes comprehensive test coverage with updated Bazel configuration.

Changes

Ordered Window Executor

Layer / File(s)	Summary
Executor types and initialization refactor `pkg/executor/windows/pipelined_window.go`	`OrderedWindowExec` struct embeds `*PipelinedWindowExec`. `PipelinedWindowExec.Open()` refactored to call `BaseExecutor.Open()` first, then delegate frame/buffer initialization to new `OpenSelf()` method.
BuildOrdered API entry point `pkg/executor/windows/builder.go`	New `BuildOrdered()` function builds window executors for pre-ordered child data by calling `Build(forcePipelined=true)` and type-asserting to `*PipelinedWindowExec`, wrapped in `OrderedWindowExec`. Adds `github.com/pingcap/errors` import for error handling.
Test validation and build configuration `pkg/executor/windows/window_executor_test.go`, `pkg/executor/windows/BUILD.bazel`	`TestBuildOrderedWindowExec` validates `BuildOrdered()` end-to-end: builds mock child, creates row_number window plan, asserts executor type, opens/drains chunks, verifies output. Helper `physicalWindowForTest` constructs test window plans. BUILD.bazel `windows_test` target updated with `shard_count=8` and new dependencies.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

pingcap/tidb#67993: Prior PR that moved window executors into the pkg/executor/windows subpackage and introduced windows.Build; this PR extends that foundation with BuildOrdered and executor lifecycle refactoring.

Suggested labels

size/L, approved, lgtm

Suggested reviewers

windtalker
wshwsh12
joechenrh

Poem

🐰 A window frame, so neat and tidy,
Ordered data marching by,
BuildOrdered shows the way,
Pipelined streams in grand array,
Tests confirm it works just right! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'executor: add ordered window builder' accurately reflects the main change—adding a new BuildOrdered function and OrderedWindowExec wrapper for window execution.
Description check	✅ Passed	The PR description is comprehensive and mostly complete, covering the problem statement (ref `#67989`), detailed explanation of changes, test coverage, and proper checklist selections; however, it lacks structured clarity in some sections.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

windtalker

lgtm

codecov · 2026-05-19T04:17:45Z

Codecov Report

❌ Patch coverage is 18.75000% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.3559%. Comparing base (a62a6d1) to head (4f7242d).
⚠️ Report is 9 commits behind head on master.

Additional details and impacted files

@@               Coverage Diff                @@
##             master     #68480        +/-   ##
================================================
- Coverage   77.2798%   75.3559%   -1.9239%     
================================================
  Files          2010       2018         +8     
  Lines        555326     566697     +11371     
================================================
- Hits         429155     427040      -2115     
- Misses       125251     139359     +14108     
+ Partials        920        298       -622

Flag	Coverage Δ
integration	`41.4609% <18.7500%> (+1.6679%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
dumpling	`60.4679% <ø> (ø)`
parser	`∅ <ø> (∅)`
br	`49.9534% <ø> (-13.0557%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

hawkingrei · 2026-05-19T05:27:28Z

/retest

ti-chi-bot · 2026-05-19T06:30:55Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tangenta, windtalker

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [tangenta,windtalker]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

ti-chi-bot · 2026-05-19T06:31:01Z

[LGTM Timeline notifier]

Timeline:

2026-05-19 04:07:13.587816096 +0000 UTC m=+237163.091946762: ☑️ agreed by windtalker.
2026-05-19 06:30:59.8149476 +0000 UTC m=+245789.319078276: ☑️ agreed by tangenta.

hawkingrei · 2026-05-19T07:43:41Z

/retest

hawkingrei · 2026-05-19T08:18:02Z

/retest

hawkingrei · 2026-05-19T09:10:53Z

/retest

hawkingrei · 2026-05-19T09:34:03Z

/retest

ti-chi-bot · 2026-05-19T13:35:42Z

@hawkingrei: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-integration-realcluster-test-next-gen	`4f7242d`	link	unknown	`/test pull-integration-realcluster-test-next-gen`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

hawkingrei · 2026-05-19T13:57:28Z

/retest

hawkingrei added 2 commits May 19, 2026 11:40

executor: add stream window builder

f00db4e

executor: rename ordered window helper

4f7242d

ti-chi-bot Bot added release-note-none Denotes a PR that doesn't merit a release note. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 19, 2026

windtalker approved these changes May 19, 2026

View reviewed changes

ti-chi-bot Bot added approved needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 19, 2026

hawkingrei requested review from AilinKid and guo-shaoge May 19, 2026 05:55

tangenta approved these changes May 19, 2026

View reviewed changes

ti-chi-bot Bot added lgtm and removed needs-1-more-lgtm Indicates a PR needs 1 more LGTM. labels May 19, 2026

ti-chi-bot Bot merged commit 64e215c into pingcap:master May 19, 2026
42 of 43 checks passed

hawkingrei deleted the exec-ordered-window-only branch May 19, 2026 14:06

Conversation

hawkingrei commented May 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What problem does this PR solve?

What changed and how does it work?

Check List

Release note

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

windtalker left a comment

Choose a reason for hiding this comment

Uh oh!

codecov Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

hawkingrei commented May 19, 2026

Uh oh!

ti-chi-bot Bot commented May 19, 2026

Uh oh!

ti-chi-bot Bot commented May 19, 2026

[LGTM Timeline notifier]

Uh oh!

hawkingrei commented May 19, 2026

Uh oh!

hawkingrei commented May 19, 2026

Uh oh!

hawkingrei commented May 19, 2026

Uh oh!

hawkingrei commented May 19, 2026

Uh oh!

ti-chi-bot Bot commented May 19, 2026

Uh oh!

hawkingrei commented May 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

hawkingrei commented May 19, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented May 19, 2026 •

edited

Loading

codecov Bot commented May 19, 2026 •

edited

Loading